Skip to content

frontend: support "casting" between MLIR types #1825

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jun 12, 2025
Merged

frontend: support "casting" between MLIR types #1825

merged 1 commit into from
Jun 12, 2025
+335 −36

Conversation

AlexanderViand
Copy link
Collaborator

@AlexanderViand AlexanderViand commented May 16, 2025 •
edited
Loading

Tries to address the use case presented in #1789 though it does so not via type hints but via adding support for casts:

def sub_bytes(x: Secret[I8]):
    x0 = I1((x >> 7) & 1)
    x1 = I1((x >> 6) & 1)
    x2 = I1((x >> 5) & 1)
    x3 = I1((x >> 4) & 1)
    ...

Still WIP:

@AlexanderViand AlexanderViand force-pushed the frontend-casts branch 2 times, most recently from 24e3542 to b029d06 Compare May 20, 2025 01:55
@AlexanderViand AlexanderViand force-pushed the frontend-casts branch 5 times, most recently from 62a87c0 to e30e535 Compare June 9, 2025 22:07
@AlexanderViand AlexanderViand marked this pull request as ready for review June 9, 2025 22:09
@AlexanderViand AlexanderViand requested a review from asraa June 9, 2025 22:09
@AlexanderViand AlexanderViand force-pushed the frontend-casts branch 2 times, most recently from 57ea506 to 0e41744 Compare June 9, 2025 22:22
@AlexanderViand
Copy link
Collaborator Author

AlexanderViand commented Jun 9, 2025 •
edited
Loading

@j2kun / @asraa: I see the cggi-frontend tests fail when doing bazel test //frontend:cggi_test but they work fine when running directly (python frontend/cggi_test.py). I'd guess this isn't related to this PR but to running CGGI from python under bazel more generally, since it happens even with the standard CGGI test (which wasn't enabled in bazel before)...

The error message I get when running with bazel is

HEIR Error at /home/sdp/.cache/bazel/_bazel_sdp/e1a6590193554bff6e6c8ff665423111/sandbox/processwrapper-sandbox/10673/execroot/_main/bazel-out/k8-dbg/bin/frontend/cggi_test.runfiles/_main/frontend/cggi_test.py (11): HEIR compilation for function foo failed:
running `heir-opt --canonicalize --mlir-to-cggi --mlir-print-debuginfo --view-op-graph` produced these errors:
ERROR: Can't open input file `/home/sdp/.cache/bazel/_bazel_sdp/e1a6590193554bff6e6c8ff665423111/sandbox/processwrapper-sandbox/10673/execroot/_main/bazel-out/k8-dbg/bin/frontend/cggi_test.runfiles/_main/techmap.v' for reading: No such file or directory

@j2kun
Copy link
Collaborator

j2kun commented Jun 11, 2025

@j2kun / @asraa: I see the cggi-frontend tests fail when doing bazel test //frontend:cggi_test but they work fine when running directly (python frontend/cggi_test.py). I'd guess this isn't related to this PR but to running CGGI from python under bazel more generally, since it happens even with the standard CGGI test (which wasn't enabled in bazel before)...

The error message I get when running with bazel is

HEIR Error at /home/sdp/.cache/bazel/_bazel_sdp/e1a6590193554bff6e6c8ff665423111/sandbox/processwrapper-sandbox/10673/execroot/_main/bazel-out/k8-dbg/bin/frontend/cggi_test.runfiles/_main/frontend/cggi_test.py (11): HEIR compilation for function foo failed:
running `heir-opt --canonicalize --mlir-to-cggi --mlir-print-debuginfo --view-op-graph` produced these errors:
ERROR: Can't open input file `/home/sdp/.cache/bazel/_bazel_sdp/e1a6590193554bff6e6c8ff665423111/sandbox/processwrapper-sandbox/10673/execroot/_main/bazel-out/k8-dbg/bin/frontend/cggi_test.runfiles/_main/techmap.v' for reading: No such file or directory

The techmap error looks like it's an issue with your build file. You'd need something like data = ["@heir//lib/Transforms/YosysOptimizer/yosys:techmap.v"], but even then it should be looking for this file in HEIR_YOSYS_SCRIPTS_DIR which may need to be configured properly (it's in heir_cli and tools/BUILD...).

Ignoring that, it is true that we can't enable CGGI tests right now because we don't have a frontend-compatible backend for it. HEIR doesn't have a CGGI backend via OpenFHE's binfhe yet, and the frontend doesn't support rust for tfhe-rs yet (nor Python for jaxite). @raghav198 has a draft from his paper in https://github.com/raghav198/heir/tree/cggi-to-openfhe-draft, but I doubt we need full CGGI support in the frontend for this PR. Can it wait?

@AlexanderViand
Copy link
Collaborator Author

AlexanderViand commented Jun 11, 2025 •
edited
Loading

Ignoring that, it is true that we can't enable CGGI tests right now because we don't have a frontend-compatible backend for it.

Yep, but this is using the CleartextBackend (so just testing down to cggi dialect) and works fine when being called outside the bazel environment, so these tests should work even w/o CGGI backend support.

I'll look into the BUILD file thing you mentioned!
EDIT: Looks like it was indeed just missing the HEIR_YOSYS_SCRIPTS_DIR (and HEIR_ABC_BINARY) paths in testing.bzl, thanks for the hint!

@AlexanderViand
Copy link
Collaborator Author

Tests pass w/ bazel now, so should be good to review :)

j2kun
j2kun approved these changes Jun 11, 2025
View reviewed changes
Copy link
Collaborator

@j2kun j2kun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, a few nits

@AlexanderViand
frontend: allow use of MLIR Types as "casts"
1dd2708 
* allow cleartext backend for CGGI
* allow types.Boolean where types.Integer is expected
* add support for `>>`/arith.shrsi
* better support for `MLIRType` when running vanilla Python
@j2kun j2kun added the pull_ready Indicates whether a PR is ready to pull. The copybara worker will import for internal testing label Jun 12, 2025
@copybara-service copybara-service bot merged commit 79f468d into google:main Jun 12, 2025
10 checks passed
j2kun added a commit to j2kun/heir that referenced this pull request Jun 14, 2025
@j2kun
types.py: use match/case
e767a18 
The main issues not allowing this to be done in the review for google#1825:

- Bare words are treated as a variable capture unless scoped (with a
  module prefix, e.g., `mt.I1`) or instantiated (`I1()`).
- The "MLIRType" values passed to these functions are not actually
  instances of MLIRType (the type hint is still incorrect in this PR),
  but rather instances of `class`. We cannot rectify this by
  instantiating the MLIRType to simplify the match syntax (e.g., use
  `I1()` instead of `mt.I1`) because google#1825 added a member variable
  on MLIRType so the type class could be used as a cast.

Possibly an improvement would be to add a no-value constructor to
MLIRType so that `I1()` can be instantiated as a standalone type,
and then call these helpers with actual instances of MLIRType rather
than class constructor objects.
@j2kun j2kun mentioned this pull request Jun 14, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@j2kun j2kun j2kun approved these changes

@asraa asraa Awaiting requested review from asraa

Assignees
No one assigned
Labels
pull_ready Indicates whether a PR is ready to pull. The copybara worker will import for internal testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@AlexanderViand @j2kun